@chengzhuzhang @czender I moved the land variable updates to this PR from #694, since they were beginning to get outside the scope of simply adding an example cfg.

I've added three test case cfgs here. Results as of latest commits:

Note: explicit list = "FSH,RH2M,LAISHA,LAISUN,QINTR,QOVER,QRUNOFF,QSOIL,QVEGE,QVEGT,SOILWATER_10CM,TSA,H2OSNO,TOTLITC,CWDC,SOIL1C,SOIL2C,SOIL3C,SOIL4C,WOOD_HARVESTC,TOTVEGC,NBP,GPP,AR,HR"

For the last row of the table, we can see in /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_undefined_variables_output/test_pr697_20250326_try2/v3.LR.historical_0051/post/scripts/ts_lnd_monthly_glb_1985-1989-0005.o717127:

ncap2: ERROR 1 dimensions of TLAKE_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of LAKEICEFRAC_tmp belong to template Internally generated template but 1 dimensions do not
Wed Mar 26 14:50:21 CDT 2025: Global and regional statistics output/TOTVEGN_198501_198912.nc
Wed Mar 26 14:50:21 CDT 2025: Global and regional statistics output/TKE1_198501_198912.nc
Wed Mar 26 14:50:22 CDT 2025: Global and regional statistics output/QCHARGE_198501_198912.nc
Wed Mar 26 14:50:22 CDT 2025: Global and regional statistics output/H2OSFC_198501_198912.nc
Wed Mar 26 14:50:22 CDT 2025: Global and regional statistics output/FPG_198501_198912.nc
Wed Mar 26 14:50:22 CDT 2025: Global and regional statistics output/DWT_CONV_PFLUX_GRC_198501_198912.nc
ncap2: ERROR 1 dimensions of PCT_LANDUNIT_tmp belong to template Internally generated template but 1 dimensions do not
Wed Mar 26 14:50:23 CDT 2025: Global and regional statistics output/AR_198501_198912.nc
Wed Mar 26 14:50:23 CDT 2025: Global and regional statistics output/ALTMAX_198501_198912.nc
ncap2: ERROR 1 dimensions of SOILICE_ICE_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of SOILLIQ_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of H2OSOI_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of SOILLIQ_ICE_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of TSOI_ICE_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of T_SCALAR_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of W_SCALAR_tmp belong to template Internally generated template but 1 dimensions do not
ncclimo: ERROR Failed in global and regional statistics. cmd_stt[8] failed. Debug this:
 OMP_PROC_BIND=false ncap2 -h -O -s '*rgn_nbr=3;defdim("rgn",rgn_nbr);*W_SCALAR_tmp=0.0f*W_SCALAR.avg($lat,$lon);*W_SCALAR_rgn[time,rgn]=W_SCALAR_tmp;W_SCALAR_rgn@coordinates="region_name";*lat_area=lat+0.0*area;*msk_sth=0*lat_\
area.int();delete_miss(msk_sth);*msk_nrt=0*lat_area.int();delete_miss(msk_nrt);*idx_glb=0;*idx_nrt=1;*idx_sth=2;*rgn_len=19;defdim("rgn_len",rgn_len);region_name[rgn,rgn_len]=" ";region_name(idx_glb,0:5)="Global";region_name(id\
x_nrt,:)="Northern Hemisphere";region_name(idx_sth,:)="Southern Hemisphere";if(0) region_name@long_name="W_SCALAR timeseries array contains area-weighted sums over these regions"; else region_name@long_name="W_SCALAR timeseries\
 array contains area-weighted averages over these regions";where(lat_area < 0.0) msk_sth=1; elsewhere msk_nrt=1;W_SCALAR_rgn(:,idx_glb)=((W_SCALAR*area*landfrac).avg($lat,$lon)/(area*landfrac).avg($lat,$lon)).float();W_SCALAR_r\
gn(:,idx_nrt)=((W_SCALAR*area*landfrac*msk_nrt).avg($lat,$lon)/(area*landfrac*msk_nrt).avg($lat,$lon)).float();W_SCALAR_rgn(:,idx_sth)=((W_SCALAR*area*landfrac*msk_sth).avg($lat,$lon)/(area*landfrac*msk_sth).avg($lat,$lon)).flo\
at();if(0){W_SCALAR_rgn(:,idx_glb)=W_SCALAR_rgn(:,idx_glb)*(area*landfrac).total($lat,$lon).float()*1.0f;W_SCALAR_rgn(:,idx_nrt)=W_SCALAR_rgn(:,idx_nrt)*(area*landfrac*msk_nrt).total($lat,$lon).float()*1.0f;W_SCALAR_rgn(:,idx_s\
th)=W_SCALAR_rgn(:,idx_sth)*(area*landfrac*msk_sth).total($lat,$lon).float()*1.0f;}W_SCALAR=W_SCALAR_rgn;push(&W_SCALAR@cell_methods," area: mean");if(exists(time_bnds)) time_bnds=time_bnds;if(exists(time_bounds)) time_bounds=t\
ime_bounds;valid_area_per_gridcell=area*landfrac;' output/W_SCALAR_198501_198912.nc output/W_SCALAR_198501_198912.nc

Please respond to my last comment on #694 (comment) here not there. I think we can get this to work with the current NCO, and that I can implement a non-kludgy solution in the next NCO.

Traceback (most recent call last):
  File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.0rc13_chrysalis/lib/python3.10/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 607, in make_plot_pdfs
    plot_generic(ax, xlim, exps, plot_name, rgn)
  File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.0rc13_chrysalis/lib/python3.10/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 437, in plot_generic
    plot(ax, xlim, exps, param_dict, rgn)
  File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.0rc13_chrysalis/lib/python3.10/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 464, in plot
    var = param_dict["var"](exp)
  File "/lcrc/soft/climate/e3sm-unified/base/envs/e3sm_unified_1.11.0rc13_chrysalis/lib/python3.10/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 432, in <lambda>
    "var": lambda exp: np.array(exp["annual"][var_name][0]),
KeyError: 'all'

cd ~/ez/zppy-interfaces
git status
# Check for uncommitted changes
git fetch upstream
git checkout -b fix_plots_lnd upstream/main
# Make edits to variable handling
git add -A
pre-commit run --all-files
git commit -m "Fix plots_lnd"
conda clean --all --y
conda env create -f conda/dev.yml -n zi_plots_lnd_20250326
conda activate zi_plots_lnd_20250326
pip install .
pytest tests/unit/global_time_series/*.py
# 10 passed in 24.05s

# Make edits to variable handling
# Update tests/integration/utils.py to use dev environment for zppy-interfaces
git add -A
pre-commit run --all-files
git commit -m "Fix variable names and use newer zi"
python tests/integration/utils.py
pip install .

zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_chrysalis.cfg
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_all_land_variables_chrysalis.cfg 
zppy -c tests/integration/generated/test_min_case_global_time_series_viewers_undefined_variables_chrysalis.cfg

Results:

So for viewers_all_land_variables_chrysalis.cfg, I guess the design decision is then do we (A) only add rows for plots that actually have data? or (B) assume the user really wanted all 354 rows regardless if there was data or not?

I would assume (B), but that may take some time to figure out how to implement.

As for viewers_undefined_variables_chrysalis.cfg, it appears the variable name changes @czender suggested do not actually seem to change anything. Output of /lcrc/group/e3sm/ac.forsyth2/zppy_min_case_global_time_series_viewers_undefined_variables_output/test_pr697_20250326_try3/v3.LR.historical_0051/post/scripts/ts_lnd_monthly_glb_1985-1989-0005.o717150:

ncap2: ERROR 1 dimensions of PCT_LANDUNIT_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of TLAKE_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of H2OSOI_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of SOILLIQ_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of T_SCALAR_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of W_SCALAR_tmp belong to template Internally generated template but 1 dimensions do not
ncclimo: ERROR Failed in global and regional statistics. cmd_stt[8] failed. Debug this:
 OMP_PROC_BIND=false ncap2 -h -O -s '*rgn_nbr=3;defdim("rgn",rgn_nbr);*W_SCALAR_tmp=0.0f*W_SCALAR.avg($lat,$lon);*W_SCALAR_rgn[time,rgn]=W_SCALAR_tmp;W_SCALAR_rgn@coordinates="region_name";*lat_area=lat+0.0*area;*msk_sth=0*lat_\
area.int();delete_miss(msk_sth);*msk_nrt=0*lat_area.int();delete_miss(msk_nrt);*idx_glb=0;*idx_nrt=1;*idx_sth=2;*rgn_len=19;defdim("rgn_len",rgn_len);region_name[rgn,rgn_len]=" ";region_name(idx_glb,0:5)="Global";region_name(id\
x_nrt,:)="Northern Hemisphere";region_name(idx_sth,:)="Southern Hemisphere";if(0) region_name@long_name="W_SCALAR timeseries array contains area-weighted sums over these regions"; else region_name@long_name="W_SCALAR timeseries\
 array contains area-weighted averages over these regions";where(lat_area < 0.0) msk_sth=1; elsewhere msk_nrt=1;W_SCALAR_rgn(:,idx_glb)=((W_SCALAR*area*landfrac).avg($lat,$lon)/(area*landfrac).avg($lat,$lon)).float();W_SCALAR_r\
gn(:,idx_nrt)=((W_SCALAR*area*landfrac*msk_nrt).avg($lat,$lon)/(area*landfrac*msk_nrt).avg($lat,$lon)).float();W_SCALAR_rgn(:,idx_sth)=((W_SCALAR*area*landfrac*msk_sth).avg($lat,$lon)/(area*landfrac*msk_sth).avg($lat,$lon)).flo\
at();if(0){W_SCALAR_rgn(:,idx_glb)=W_SCALAR_rgn(:,idx_glb)*(area*landfrac).total($lat,$lon).float()*1.0f;W_SCALAR_rgn(:,idx_nrt)=W_SCALAR_rgn(:,idx_nrt)*(area*landfrac*msk_nrt).total($lat,$lon).float()*1.0f;W_SCALAR_rgn(:,idx_s\
th)=W_SCALAR_rgn(:,idx_sth)*(area*landfrac*msk_sth).total($lat,$lon).float()*1.0f;}W_SCALAR=W_SCALAR_rgn;push(&W_SCALAR@cell_methods," area: mean");if(exists(time_bnds)) time_bnds=time_bnds;if(exists(time_bounds)) time_bounds=t\
ime_bounds;valid_area_per_gridcell=area*landfrac;' output/W_SCALAR_198501_198912.nc output/W_SCALAR_198501_198912.nc

@czender For my last comment, I used --xcl_var -v .PCT_LANDUNIT,.TLAKE,.LAKEICEFRAC,.SOILLIQ_ICE,.W_SCALAR,.T_SCALAR,.SOILICE_ICE,.SOILPSI,.O_SCALAR,.H2OSOI as suggested in #694 (comment).

the design decision is then do we (A) only add rows for plots that actually have data? or (B) assume the user really wanted all 354 rows regardless if there was data or not? I would assume (B), but that may take some time to figure out how to implement.

Could conceivably be implemented with a check if var is empty in coupled_global_plotting.py:

        else:
            year = np.array(exp["annual"]["year"]) + exp["yoffset"]
            var = param_dict["var"](exp)
        extreme_values.append(np.amax(var))
        extreme_values.append(np.amin(var))

And if so, raise an exception so it will get caught at:

                try:
                    plot_name = plot_list[counter]
                    plot_generic(ax, xlim, exps, plot_name, rgn)
                    valid_plots.append(plot_name)
                except Exception:
                    traceback.print_exc()
                    logger.error(
                        f"plot_generic failed. Invalid plot={plot_name}, rgn={rgn}"
                    )
                    invalid_plots.append(plot_name)

the design decision is then do we (A) only add rows for plots that actually have data? or (B) assume the user really wanted all 354 rows regardless if there was data or not? I would assume (B), but that may take some time to figure out how to implement.

I think (A) makes more sense, and should make the page easier to navigate if only a small set of variables are available.

I think (A) makes more sense, and should make the page easier to navigate if only a small set of variables are available.

Ok, that is good. I think (B) would be trickier to implement than I initially thought. By the time, coupled_global realizes there's no data to plot, the plot has already been constructed

Traceback (most recent call last):
  File "/gpfs/fs1/home/ac.forsyth2/miniforge3/envs/zi_plots_lnd_20250326/lib/python3.12/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 609, in make_plot_pdfs
    plot_generic(ax, xlim, exps, plot_name, rgn)
  File "/gpfs/fs1/home/ac.forsyth2/miniforge3/envs/zi_plots_lnd_20250326/lib/python3.12/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 437, in plot_generic
    plot(ax, xlim, exps, param_dict, rgn)
  File "/gpfs/fs1/home/ac.forsyth2/miniforge3/envs/zi_plots_lnd_20250326/lib/python3.12/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 464, in plot
    var = param_dict["var"](exp)
          ^^^^^^^^^^^^^^^^^^^^^^
  File "/gpfs/fs1/home/ac.forsyth2/miniforge3/envs/zi_plots_lnd_20250326/lib/python3.12/site-packages/zppy_interfaces/global_time_series/coupled_global_plotting.py", line 432, in <lambda>
    "var": lambda exp: np.array(exp["annual"][var_name][0]),
                                ~~~~~~~~~~~~~^^^^^^^^^^
KeyError: 'WIND'

Sorry, I didn't realize my typo; I got everything backwards. I meant to say "I would assume (A)" and "I think (A) would be trickier".

Ok, this is going to take some debugging work to implement (A).

Ok, this is going to take some debugging work to implement (A).

Is the problem occurring in the plotting functionality or in the viewer component? It might be easier to debug if we isolate which part is causing the issue.

Ah fantastic, the fix was actually very easy, just in a different place than I was thinking -- we can catch invalid variables much earlier on in the globalAnnual computation: see E3SM-Project/zppy-interfaces@e7d3466. That generates a working viewer

Is the problem occurring in the plotting functionality
we can catch invalid variables much earlier on in the globalAnnual computation

I think this issue stems from the fact than in the original 8-plot single-page PDF, we wanted to keep empty placeholder plots if ocean data was missing. Which is why the code as-is would only warn of variables that couldn't have a globalAnnual calculated rather than removing them completely. But it seems our use cases nowadays don't align with that -- i.e., no point having an empty viewer link.

we wanted to keep empty placeholder plots if ocean data was missing.

I believe that was a holdover from when we had atmosphere_only, but that parameter is now deprecated. I think the correct path forward is indeed to never make empty plots for missing data, so the latest changes in zppy-interfaces seem fine to me.

@czender That just leaves the one remaining issue of figuring out why NCO is failing trying to exclude certain variables. Did I implement --xcl_var -v .PCT_LANDUNIT,.TLAKE,.LAKEICEFRAC,.SOILLIQ_ICE,.W_SCALAR,.T_SCALAR,.SOILICE_ICE,.SOILPSI,.O_SCALAR,.H2OSOI incorrectly?

we wanted to keep empty placeholder plots if ocean data was missing.

I believe that was a holdover from when we had atmosphere_only, but that parameter is now deprecated. I think the correct path forward is indeed to never make empty plots for missing data, so the latest changes in zppy-interfaces seem fine to me.

Maybe I didn't fully understand, but, for the original 8, I thought we want the position of the variables fairly static.
For the newly added plot_atm, and plot_land, we perhaps don't want to empty plots on pdfs, because the land group prefers to use viewer to organize hundreds of plots.

But this is more of a cosmetic thing, we can review the results and then decide.

for the original 8, I thought we want the position of the variables fairly static.

In the original-8 format, there is only one atm plot after the 3 ocn plots. That means even skipping the 3 ocn plots, the first 4 atm plots remain in the same position.

I'm actually struggling to see how we'd get in the situation of having empty ocn plots now, since atmosphere_only has been deprecated and global_time_series will force a mpas_analysis dependency if ocn variables are requested.

this is more of a cosmetic thing

Yes, I think all things considered, it's not worth the time to try to implement this two different ways based on if we're doing the original 8 plots vs all 354 Land variables. And frankly, it makes sense to not waste space on plots that have no data.

@forsyth2 You implemented the exclusion command in accord with my instructions. However, I think my instructions that prefixing the variable names with a period would work were incorrect. Please try without the period, i.e., with
--xcl_var -v PCT_LANDUNIT,TLAKE,LAKEICEFRAC,SOILLIQ_ICE,W_SCALAR,T_SCALAR,SOILICE_ICE,SOILPSI,O_SCALAR,H2OSOI
Be aware that this may mean that this exclusion will break on non-default ELM h0 files and EAM file etc.

@czender It turns out I had to add a few more missing variables to the exclusion list. It seems to be working now without the periods.

will break on non-default ELM h0 files and EAM file etc.

Can you explain this more? I'm wondering if we should allow users to specify their variable exclusion list via a parameter (and have this list as a default). I'm just not sure which variables each simulation output may contain.

The exclusion list without periods will break if the input lacks any of the variables in the exclusion list. Your list is based on default ELM v0 so it will break on EAM h0 or ELM h1 default outputs. The purpose of the periods is to make each variable in the exclusion list optional in the input files. I think your previous list with the periods was breaking because it was incomplete, not because of the periods. Please retry the complete list with the periods. If it works on ELM h0, then you can add to it the multilevel fields that are in ELM h1, or EAM h0, or whatever, and nothing will break.

Please retry the complete list with the periods.

@czender I'll try adding those back in too. It took a little over 2 hours to run the full global_time_series for Land's 354 variables, but I think it will be sufficient just to re-test the ts task with the periods included.

@chengzhuzhang Ok it appears everything is working correctly now. Please let me know if you are happy with these Viewers and if so, I'll merge.

@forsyth2 thank you for the results. The viewer looks okay. Through I don't seam to find the original 8 panel plots. Are they just not turned on in the .cfg being tested?

Through I don't seam to find the original 8 panel plots. Are they just not turned on in the .cfg being tested?

Correct, though I did actually run the original_8_no_ocn cfg yesterday: viewer here. You can see it plots the 5 atm plots and nothing else.

Through I don't seam to find the original 8 panel plots. Are they just not turned on in the .cfg being tested?

Correct, though I did actually run the original_8_no_ocn cfg yesterday: viewer here. You can see it plots the 5 atm plots and nothing else.

Do you have a case both original pdf and the viewer results are available?

Do you have a case both original pdf and the viewer results are available?

@chengzhuzhang No, but I can construct one. Note that this would take another 2+ hours for global_time_series to run again. Also, you need to specify what type of original PDF you want -- 1) atm plots only, 2) both atm and ocn plots, 3) atm and ocn plots requested but no ocn data available [I'm not sure how to force this scenario other than simply not running mpas_analysis but that will cause global_time_series to not run because it's missing a dependency]

Please retry the complete list with the periods.

@czender No, the ncap2 errors return if I add the periods:

ncap2: ERROR 1 dimensions of PCT_LANDUNIT_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of SOILPSI_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of H2OSOI_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of TSOI_tmp belong to template Internally generated template but 1 dimensions do not
ncap2: ERROR 1 dimensions of W_SCALAR_tmp belong to template Internally generated template but 1 dimensions do not

The line used in the bash script was: --xcl_var -v .H2OSOI,.LAKEICEFRAC,.O_SCALAR,.PCT_LANDUNIT,.PCT_NAT_PFT,.SOILICE,.SOILICE_ICE,.SOILLIQ,.SOILLIQ_ICE,.SOILPSI,.T_SCALAR,.TLAKE,.TSOI,.TSOI_ICE,.W_SCALAR \

@forsyth2 I do not understand why the periods work for me and not for you/zppy. I suggest you go back to what works, and firewall that exclusion list so it is only applied when the input model is ELM. This should prevent the exclusion list from breaking the EAM timeseries. I'll work on getting ncclimo to do the right thing and select only 2D variables for timeseries to begin with. Then you can remove the kludgy exclusion list from zppy entirely after that.

My understanding was some, like Chris and Wuyin, would use the non-make_viewers workflow because they wanted the original 8 plots, and others, like the Land team, would use the make_viewers workflow. It was not my understanding that someone would want everything turned on.

I think how it works it that, the coupled group will run the comprehensive zppy .cfg, and land group will examine the output, without needing to run zppy again on the same simulation... pinging @golaz or @wlin7

Why is c["plots_original"] evaluating as empty/false?

Well this was simple; I had set it as plots_original="" in the cfg I ran....

In any case, running a cfg that has both plots_original and plots_lnd set now.

the coupled group will run the comprehensive zppy .cfg, and land group will examine the output, without needing to run zppy again on the same simulation

Is that specific to Coupled runs, because Coupled by definition includes eveyrthing? I was under the impression the different groups ran their own simulations (e.g., BGC has distinct simulations from WaterCycle) that have data on variables specific to them and therefore post-processing needs specific to them.

@forsyth2 and @chengzhuzhang The current NCO snapshot in my directories on Chrysalis and PM have a version of ncclimo that does not require an exclusion list to determine the 2D variables to produce global/regional timeseries for. In other words, it eliminates the zppy kludge that specifically excludes EAM U, and the more recent kludges you/we are adding for ELM. If ncclimo is called without an explicit extraction list, and regional timeseries are requested, then this new snapshot uses a custom routine to generate a list of horizontal-only timeseries for the splitter. Horizontal-only just means the dimensions must be either lat, lon, ncol, nCells, and [tT]ime.

To test this with zppy, invoke ncclimo with e.g., /home/ac.zender/bin_chrysalis/ncclimo --npo .... NB: The --npo flag must be the FIRST argument. Feedback welcome.

current NCO snapshot in my directories

Thanks @czender but I'm going to have to hold off on testing that for now. We need these zppy changes in for the imminent Unified release, which I don't believe is going to include the latest NCO. That is, zppy will need to work with the version of NCO in E3SM Unified 1.11.0rc13.

@chengzhuzhang I've been having difficulty getting Chrysalis nodes all afternoon, but I finally have a Viewer with both the original plots and the land plots. Unfortunately... the original plots Viewer has no working links, but there is a PDF (one plot per page) of the 5 requested atm plots.

The use case of requesting both the original plots and the component plots was simply not something built into the initial implementation of the global_time_series Viewers. This level of zppy doing it all, as part of the same cfg, is not particularly simple to execute. As one example, nrows and ncols are passed from zppy to zppy-interfaces. We'd need two versions of those parameters to produce both PDFs and viewers. Currently make_viewer forces nrows=ncols=1, because the viewer needs to link to a PDF of exactly 1 plot.

I think at this point we need to make a decision if we want to (A) raise some sort of NotYetImplementedError if a user requests both the original plots and the viewers OR (B) further delay the Unified release to get this working properly in all scenarios.

@forsyth2 I think we really need to consult with @golaz, @wlin7 and @thorntonpe for making this decision, about the desired feature of the viewer, the expected change, and time line. I suppose this will first impact the current V3 paper and related analysis work.

@forsyth2 I attempted a PR in zppy-interface (with help from Claude), does this seem reasonable to preserve the plotting feature for the original coupled 8 plots, while also support viewer for component variables?

@chengzhuzhang Thanks, let me cherry-pick that commit into E3SM-Project/zppy-interfaces#16 and re-test.

Also, I was trying to figure out how we missed the collision of use cases (i.e., wanting both a single-page original plots PDF and viewers for other variables).

In E3SM-Project/zppy-interfaces#9 (comment) (from 1/14):

Issue 3: this is a design decision question. I guess it would be best to have a specific make_viewer parameter rather than relying on a user specifying they want 1 row and 1 column in the parameters. It's possible a user would want single plot images, but not a Viewer. (Although, the inverse doesn't work; we need 1x1 images to use in the Viewer).

It appears the combined use case was never considered.

@chengzhuzhang Ok using the code from E3SM-Project/zppy-interfaces#16 (which now includes your commit from E3SM-Project/zppy-interfaces#17) & this PR, I created this viewer. It shows atm and lnd Viewers, but the original link goes to a broken link.

I see this:

cd /lcrc/group/e3sm/public_html/diagnostic_output/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_www/test_pr697_20250328_try1/v3.LR.historical_0051/global_time_series/global_time_series_1985-1995_results
ls *pdf
# v3.LR.historical_0051_glb_atm.pdf       v3.LR.historical_0051_n_atm.pdf       v3.LR.historical_0051_s_atm.pdf
# v3.LR.historical_0051_glb_lnd.pdf       v3.LR.historical_0051_n_lnd.pdf       v3.LR.historical_0051_s_lnd.pdf
# v3.LR.historical_0051_glb_original.pdf  v3.LR.historical_0051_n_original.pdf  v3.LR.historical_0051_s_original.pdf

So it may be a permissions issue? I will take a look. Once I get that working, then we might actually have the issue resolved.

Nevertheless, I want to make sure no further specifications are missing:

• Is the original-8 (5 without ocean) plot PDF the only case where we don't proceed with a Viewer? If so, it's fine to have this hard-coded, but there's a bigger issue here if users will want atm plots in a PDF and lnd plots in a Viewer, for example. Or both PDFs and Viewers for their plots.
• Is there ever a case where we want the original plots displayed in a Viewer then?

I also want to highlight how this is an example of exponential complexity in zppy. Different use cases combined together (e.g., vars="" or an explicit list, plots_lnd="all" or an explicit list, plot PDFs and/or viewers) create a large testing space. In this PR, I've added the following "min-case" cfgs:

tests/integration/template_min_case_global_time_series_original_8_missing_ocn.cfg
tests/integration/template_min_case_global_time_series_viewers.cfg
tests/integration/template_min_case_global_time_series_viewers_all_land_variables.cfg
tests/integration/template_min_case_global_time_series_viewers_original_8.cfg
tests/integration/template_min_case_global_time_series_viewers_original_atm_plus_land.cfg
tests/integration/template_min_case_global_time_series_viewers_undefined_variables.cfg

That is, a single pull request has generated 6 independent testing configurations (and there are certainly uncovered scenarios). I have been running these and looking at results manually -- for my last comment (#697 (comment)) I only tested one: template_min_case_global_time_series_viewers_original_atm_plus_land.cfg. (Automating these specific test cases is the scope of #698, which is looking quite involved.)

I think it's a good thing that we can provide users with so much flexibility/customizability but there are certainly challenges on the design/testing side.

but the original link goes to a broken link.

Ah, https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_www/test_pr697_20250328_try1/v3.LR.historical_0051/global_time_series/global_time_series_1985-1995_results/ links to global_time_series_1985-1995_results/v3.LR.historical_0051_glb_original.pdf when it should link to only v3.LR.historical_0051_glb_original.pdf: https://web.lcrc.anl.gov/public/e3sm/diagnostic_output/ac.forsyth2/zppy_min_case_global_time_series_viewers_original_atm_plus_land_www/test_pr697_20250328_try1/v3.LR.historical_0051/global_time_series/global_time_series_1985-1995_results/v3.LR.historical_0051_glb_original.pdf works.

Ok, so the link needs to be fixed

Ok after adding my latest changes: viewer 20250328_try2 works -- lnd links to lnd plot links in a table while original links to a single page PDF with 5 plots. Unfortunately, the lnd plots are now distorted; the aspect ratio appears to be off. This problem exists in viewer 20250328_try1 as well, but not in my last run from yesterday, viewer 20250327_try10.

Ok viewer 20250328_try4 appears to have the dimensions issue fixed. I'll run the other cfg's to make sure we don't run into other issues.

@chengzhuzhang This is ready for review. Unit tests pass on both zppy-interfaces and zppy. The only thing that looks a little off to me is how the original plots are linked when make_viewer=True but none of the component plots are requested. It's really just a cosmetic issue though; it works.

To review functionality (scroll right to see more of the table):

To review code changes:

• https://github.com/E3SM-Project/zppy-interfaces/pull/16/files (this includes the commit cherry-picked from your PR: Make original plots always use multi-plot layout zppy-interfaces#17)
• https://github.com/E3SM-Project/zppy/pull/697/files

Also, after the E3SM Unified release, we'll have to make a PR to incorporate @czender's NCO updates (see #697 (comment)).

I'm not sure if the use case that includes all land ts var, and the original plots, is listed in this table? I think that's the configuration we will have in the example .cfg?

@chengzhuzhang Yes that's correct; that would have been a row like:

And indeed that's the example cfg in https://github.com/E3SM-Project/zppy/pull/694/files:

[ts]
  [[ lnd_monthly_glb ]]
  vars = ""

[global_time_series]
plots_atm = "TREFHT,AODDUST"
plots_lnd = "all"

So, in that case, I'll merge this PR, test #694 with these changes, and then merge that one too.

The current NCO snapshot in my directories on Chrysalis and PM have a version of ncclimo that does not require an exclusion list to determine the 2D variables to produce global/regional timeseries for.

@czender I made #704 to make sure we return to this after the Unified release.